Skip to content

feat: Add clamp() method in TimeTrait#10143

Open
patel-vansh wants to merge 4 commits intocodeigniter4:4.8from
patel-vansh:feat/clamp-method
Open

feat: Add clamp() method in TimeTrait#10143
patel-vansh wants to merge 4 commits intocodeigniter4:4.8from
patel-vansh:feat/clamp-method

Conversation

@patel-vansh
Copy link
Copy Markdown
Contributor

Description
This PR adds a clamp($start, $end) in TimeTrait class.

This method works like this:

  • If current time instance is before $start, then a new $start instance is returned.
  • If current time instance is after $end, then a new $end instance is returned.
  • For the third case, where current time instance is between $start and $end, currently, $this is returned. I was unsure about this particular case initially, as should a new instance be returned or the current one? Will change according to the suggestion.

This method is useful in places where user input should be between predefined range.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@github-actions github-actions Bot added the 4.8 PRs that target the `4.8` branch. label Apr 25, 2026
Comment thread system/I18n/TimeTrait.php
Comment on lines +873 to +883
if ($start instanceof DateTimeInterface && ! $start instanceof self) {
$start = static::createFromInstance($start, $this->locale);
} elseif (is_string($start)) {
$start = new static($start, $this->getTimezone(), $this->locale);
}

if ($end instanceof DateTimeInterface && ! $end instanceof self) {
$end = static::createFromInstance($end, $this->locale);
} elseif (is_string($end)) {
$end = new static($end, $this->getTimezone(), $this->locale);
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like this way. But I didn't found any current method that would cleanly return the static instance from either DateTimeInterface, self and string. So, maybe create a method for this?

Copy link
Copy Markdown
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced this should be added to the framework core.

I think implementing something like: between(), min(), and max() would be more useful and a better fit for this class, and that is also closer to how popular packages like Carbon and Chronos handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.8 PRs that target the `4.8` branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants